Package transactions: Always set error #2363
Open
+52
−127
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leolost2605
force-pushed
the
leolost/always-set-error
branch
from
4ff3d62 to
abebc6f
Compare
tintou
reviewed
Oct 13, 2025
leolost2605
force-pushed
the
leolost/always-set-error
branch
from
abebc6f to
d442f41
Compare
lenemter
reviewed
Oct 19, 2025
lenemter
reviewed
Oct 19, 2025
Member
|
@leolost2605 can you resolve conflicts and follow up here when you get a chance :) |
Member
Author
|
Yes I'll see if i can get it done this evening :) |
leolost2605
force-pushed
the
leolost/always-set-error
branch
from
d442f41 to
86b200d
Compare
Member
Author
|
Rebased and added some more warnings and cleanup |
false is never returned because instead an error is thrown
leolost2605
force-pushed
the
leolost/always-set-error
branch
from
2c30b7a to
5b3e10f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first three commits make sure we always set an error instead of returning false for the three package transactions. This way we make sure we never fail silently for the user. Also it simplifies not having to deal with a success variable as well.
There was also bug where != was used instead of ==. The comment said we don't want to overwrite existing errors but the code checked if there was an existing error and if there WAS it overwrote it instead of the other way round.
The fourth commit ignores the return value from the package ops since it will now always return true or set an error so it assumes success if no error is set.
And the last commit is a little cleanup that changes the backend methods to
void.This also serves as preparation for fixing cancelling in a way that we get immediate feedback when cancelling instead of just a flicker that looks like it doesn't cancel.
Can use rebase to merge.